Skip to content

Add servo flag#80

Draft
sagudev wants to merge 8 commits intoerichdongubler-mozilla:mainfrom
sagudev:servo
Draft

Add servo flag#80
sagudev wants to merge 8 commits intoerichdongubler-mozilla:mainfrom
sagudev:servo

Conversation

@sagudev
Copy link
Copy Markdown
Collaborator

@sagudev sagudev commented Feb 15, 2024

Currently moz-webgpu-cts --servo process-reports --preset merge ../../Downloads/wpute/* works on servo if all disabled: comment are replaced with disabled: true

@ErichDonGubler
Copy link
Copy Markdown
Collaborator

ErichDonGubler commented Feb 27, 2024

This hasn't actually been submitted for review, but 2 design notes on the current contents that, if changed, I could see getting upstreamed:

  1. I think it would be a closer/more honest data model to reorganize the TestScope enum into two enum fields of a struct instead. One would indicate the ecosystem being worked with (Firefox or Servo), and another would indicate whether the Public or Private directories in the chosen ecosystem. These can be combined into a reformed struct TestScope as separate fields:

    enum Browser {
        Firefox,
        Servo,
    }
    
    enum TestVisibility {
        Public,
        Private,
    }
    
    struct TestScope {
        visibility: TestVisibility,
        browser: Browser,
    }

    With all this, we could probably rename TestPath::from_fx_metadata_test to TestPath::from_metadata_test, and require a Browser to be specified as an argument.

  2. After the last point, I'd like to see servo: bool become a usage of the proposed enum Browser.1 Then, the variant can be passed to the proposed/renamed TestPath::from_metadata_test constructor from CLI.

Footnotes

  1. Perhaps defaulting to --browser=firefox, to preserve the existing CLI experience?

@sagudev sagudev mentioned this pull request May 30, 2024
2 tasks
@sagudev sagudev mentioned this pull request Jul 31, 2024
@ErichDonGubler
Copy link
Copy Markdown
Collaborator

@sagudev: RE: the disabled property not accepting anything but true: Does #143 help?

@sagudev
Copy link
Copy Markdown
Collaborator Author

sagudev commented Aug 2, 2024

@sagudev: RE: the disabled property not accepting anything but true: Does #143 help?

Looks like it should (although we rewrite all disabled with true values), but I didn't tried it yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants